-
-
Notifications
You must be signed in to change notification settings - Fork 31.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zha switch schedule update state #16621
Zha switch schedule update state #16621
Conversation
if turning switch On or Off operation was successful, then schedule state update
return | ||
|
||
self._state = 1 | ||
self.async_schedule_update_ha_state() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't do this, and neither should we change self._state
. We depend on attribute_updated
to push the state change to us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure relying on attribute_updated
is a proper channel for this kind of state updated, especially when we're triggering the change.
We do send a ZCL command and we do know the transmission hasn't timed out, in other words device has acknowledge reception of the command.
The problem with attribute_updated
it relies on attribute reporting configuration, so if for whatever reason zha.configure_reporting()
was unsuccessful, this makes the switch unusable, as I never can change the state (because the feedback loop through attribute_updated
is broken) even though I can control the switch and switch can successfully receive my commands.
attribute_updated
channel is more for externally originated events, like user toggling the switch from the button on the actual physical device.
@rcloran what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also want to implement attribute_updated()
method for fan.zha.ZhaFan
class, because it is nice for HA state to be in sync with the actual state of the device, since the fan is often operated externally from the remote/wall switch.
I'd like to understand why it is fundamentally wrong with updating self._state
in the service calls and in attribute_updated()
method. Both are executed in the event loop, so there shouldn't be any race conditions. I'm not always getting attribute report triggering the attribute_update()
with King of Fans zigbee fan, so relying solely on atribute_updated()
to update self._state
would likely render Fan inoperable from HA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems fine if we are guaranteed that command has been received.
Thank you. |
Description:
switch.zha is a non-polled entity
should_poll = False
so whenever the switch is successfully turned on or off, it should schedule state update, otherwise the state wouldn't be updated in the UIChecklist:
tox
. Your PR cannot be merged unless tests pass